Bazel: Bump java source and target language level to java 8 #6711

davido · 2019-09-29T05:49:48Z

No description provided.

rafi-kamal · 2019-10-01T16:10:37Z

BUILD

@@ -654,8 +654,8 @@ java_library(
    javacopts = select({
        "//:jdk9": ["--add-modules=jdk.unsupported"],
        "//conditions:default": [
-            "-source 7",
-            "-target 7",
+            "-source 8",


Does this mean that we are going to drop Java 7 support?

I don't think we would like to drop Java 7 support soon.

The problem is: Java 11 doesn't support building with language level 7. Moreover, Bazel switched to using JDK 11 internally, and the console is spammed with spurious warning messages:

warning: -parameters is not supported for target value 1.7. Use 1.8 or later.

in many issue trackers: [1], [2].

But I see now, that Maven build doing something similar in java/pom.xml so that this PR is incomplete:

<plugin> <artifactId>maven-compiler-plugin</artifactId> <version>3.6.1</version> <configuration> <source>1.7</source> <target>1.7</target> </configuration> </plugin>

But personally, I don't care about Maven. Would it be an option to remove Java 7 support for Bazel driven build, so to say merge this PR as is, but let Maven world untouched? That way all bazel friends in the wild wouldn't see these annoying warnings, for each and every java_proto_library rules in their own and transitive dependencies BUILD files.

For example in Gerrit Code Review, I am seeing these warnings dozen times:

During the build of protobuf itself (would be fixed in this PR)

During the build of rules_closure (transitive dependency of gerrit)

During the build of gerrit itself, should be fixed with this CL in Bazel itself

[1] bazelbuild/bazel#8772
[2] https://bugs.chromium.org/p/gerrit/issues/detail?id=11102

I see. We don't want existing JDK 7 users to be stuck in an old version of Protobuf. But since you are saying Java 11 doesn't support building language level 7 and Bazel switched to using JDK 11, does this mean there are currently no Bazel users who are using JDK 7?

I apologize, my bad. Java 11, 13 and even 14 version still support java language level 7 (byte code major version 51).

This issue is entirely a Bazel bug (incompatibility of two Java compiler options) and should be be fixed in Bazel instead. Sorry for the noise.

Having said that, you should consider to discontinue Java 7 language level support in future Protobuf releases.

The most recent JDK 13 and upcoming JDK 14 release already issue this warning. JDK 13:

$ cat A.java class A {} $ ~/pgm/jdk-13/bin/javac -source 7 -target 7 -parameters A.java warning: [options] source value 7 is obsolete and will be removed in a future release warning: [options] target value 7 is obsolete and will be removed in a future release warning: -parameters is not supported for target value 7. Use 8 or later.

JDK 14:

$ ~/pgm/jdk-14/bin/javac -source 7 -target 7 -parameters A.java warning: [options] source value 7 is obsolete and will be removed in a future release warning: [options] target value 7 is obsolete and will be removed in a future release warning: -parameters is not supported for target value 7. Use 8 or later.

davido · 2019-10-03T10:06:52Z

FTR: the proper fix in Bazel project is here.

This is a workaround to suppress annoying warning: warning: -parameters is not supported for target value 1.7. Use 1.8 or later. wenn building protobuf_java. The full story can be seen in this Bazel issue: [1]. I tried hard to avoid the need for patching Protobuf on fetch, by trying to remove Java 7 compatibility mode upstrea, that was rejected: [2]. moreover, I tried hard and spent quite some time to fix the problem in Bazel: [3], in non-invasive way, but this PR was as well rejected. The next natural approach is to just patch the Protobuf source during the fetch operation. Needless to say, that the patching during the fetch has its own disavdantages, that the patch would probably neeed to be updated on every Protobuf upgrade. But what can you do? The developers are complaining and writing the issues in every issue tracker, as here in Gerrit Code Review project: [4]. [1] protocolbuffers/protobuf#6711 [3] bazelbuild/bazel#9494 [3] protocolbuffers/protobuf#6711 [4] https://bugs.chromium.org/p/gerrit/issues/detail?id=11102

This is a workaround to suppress this annoying warning: warning: -parameters is not supported for target value 1.7. Use 1.8 or later. wenn building protobuf_java. The full story can be seen in this Bazel issue: [1]. I tried hard to avoid the need for patching Protobuf on fetch, by trying to remove Java 7 compatibility mode upstream, that was rejected: [2]. Moreover, I tried hard and spent quite some time to fix the problem in Bazel: [3], in non-invasive way, but this PR was as well rejected. The next natural approach is to just patch the Protobuf source during the fetch operation. Needless to say, that the patching during the fetch has its own disavdantages, that the patch would probably need to be updated on every Protobuf upgrade. But what can you do? The developers are complaining and filling the issues in every issue tracker, as here in Gerrit Code Review project: [4]. [1] protocolbuffers/protobuf#6711 [3] bazelbuild/bazel#9494 [3] protocolbuffers/protobuf#6711 [4] https://bugs.chromium.org/p/gerrit/issues/detail?id=11102

This is a workaround to suppress this annoying warning: warning: -parameters is not supported for target value 1.7. Use 1.8 or later. wenn building protobuf_java. The full story can be seen in this Bazel issue: [1]. I tried hard to avoid the need for patching Protobuf on fetch, by trying to remove Java 7 compatibility mode upstream, that was rejected: [2]. Moreover, I tried hard and spent quite some time to fix the problem in Bazel: [3], in non-invasive way, but this PR was as well rejected. The next natural approach is to just patch the Protobuf source during the fetch operation. Needless to say, that the patching during the fetch has its own disavdantages, that the patch would probably need to be updated on every Protobuf upgrade. But what can you do? The developers are complaining and filling the issues in every issue tracker, as here in Gerrit Code Review project: [4]. [1] bazelbuild/bazel#8772 [3] bazelbuild/bazel#9494 [3] protocolbuffers/protobuf#6711 [4] https://bugs.chromium.org/p/gerrit/issues/detail?id=11102

davido · 2020-03-15T08:27:35Z

//CC @Yannic.

This is the rejected attempt to drop the Java 7 (declared as dead dozen years ago) compatibility from Protobuf library. And that's why patching protobuf on fetch in rules_closure build tool chain is still needed: [1].

[1] bazelbuild/rules_closure#478

Yannic · 2020-03-15T12:14:39Z

There's already a special-case for JDK 9 (which, btw, results in not setting -source 7 -target 7 for JDK 9). Why don't we change all this to set -source 7 -target 7 iff JDK 7 and -source 8 -target 8 otherwise?

    javacopts = select({
        # TODO: I think this should actually be "//java:jdk7": [], "//java:jdk8": [], "//conditions:default": ["--add-modules=jdk.unsupported"] so >= JDK 9 sets --add-modules=jdk.unsupported.
        "//java:jdk9": ["--add-modules=jdk.unsupported"],
        "//conditions:default": [],
    }) + select({
        "//java:jdk7": [
            "-source 7",
            "-source 7",
        ],
        "//conditions:default": [
            "-source 8",
            "-source 8",
        ],
    })

davido · 2020-03-15T12:37:42Z

There's already a special-case for JDK 9 [...]

There no such thing as JDK9. It's dead and discontinued, so nobody really care about that configuration option.

Why don't we change all this to set -source 7 -target 7 iff JDK 7 and -source 8 -target 8 otherwise?

There is no such thing as jdk7 in Bazel: It is the dead horse and was dropped years ago:

  $ cd bazel
  $ git grep jdk7
  CHANGELOG.md:   + b95995b: Use openjdk7 as dependency for debian package of jdk7
scripts/packages/debian/BUILD:        # 0.1.2rc2-jdk7 0.1.2~rc2-jdk7
src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java:    // Allow general purpose bit 3 (data descriptor) used in jdk7 to differ.
src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java:    // Allow general purpose bit 11 (UTF-8 encoding) used in jdk7 to differ.

The whole point with backwards compatibility in protobuf, is that major byte code version:

JDK17_MAJOR_VERSION = 51

is created even if Java 8, 11, 13 and 14 and newer are used.

I'm curious, what Bazel driven project is using Protobuf and still relies on JDK 7? Can someone point me to a one single Open Source project who is doing that?

If none is doing/using that feature, can we re-consider dropping JDK7 compatibility in Protobuf project in Bazel driven build tool chain only, but keep it in Gradle and Apache Maven?

davido · 2020-03-15T13:04:44Z

I uploaded a new PR: #7306.

Yannic · 2020-03-15T13:08:09Z

There's already a special-case for JDK 9 [...]

There no such thing as JDK9. It's dead and discontinued, so nobody really care about that configuration option.

Then let's get rid of that configuration.

Why don't we change all this to set -source 7 -target 7 iff JDK 7 and -source 8 -target 8 otherwise?

There is no such thing as jdk7 in Bazel: It is the dead horse and was dropped years ago:
  $ cd bazel
  $ git grep jdk7
  CHANGELOG.md:   + b95995b: Use openjdk7 as dependency for debian package of jdk7
scripts/packages/debian/BUILD:        # 0.1.2rc2-jdk7 0.1.2~rc2-jdk7
src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java:    // Allow general purpose bit 3 (data descriptor) used in jdk7 to differ.
src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipCombinerTest.java:    // Allow general purpose bit 11 (UTF-8 encoding) used in jdk7 to differ.
The whole point with backwards compatibility in protobuf, is that major byte code version:

JDK17_MAJOR_VERSION = 51

is created even if Java 8, 11, 13 and 14 and newer are used.

I'm curious, what Bazel driven project is using Protobuf and still relies on JDK 7? Can someone point me to a one single Open Source project who is doing that?

If none is doing/using that feature, can we re-consider dropping JDK7 compatibility in Protobuf project in Bazel driven build tool chain only, but keep it in Gradle and Apache Maven?

So all this is about is binary compatibility of protobuf.jar? I'll need to think about this for a while, but, as of now, I don't think this is something we should care about a lot for Bazel.

This is a workaround to suppress this annoying warning: warning: -parameters is not supported for target value 1.7. Use 1.8 or later. wenn building protobuf_java. The full story can be seen in this Bazel issue: [1]. I tried hard to avoid the need for patching Protobuf on fetch, by trying to remove Java 7 compatibility mode upstream, that was rejected: [2]. Moreover, I tried hard and spent quite some time to fix the problem in Bazel: [3], in non-invasive way, but this PR was as well rejected. The next natural approach is to just patch the Protobuf source during the fetch operation. Needless to say, that the patching during the fetch has its own disavdantages, that the patch would probably need to be updated on every Protobuf upgrade. But what can you do? The developers are complaining and filling the issues in every issue tracker, as here in Gerrit Code Review project: [4]. [1] bazelbuild/bazel#8772 [3] bazelbuild/bazel#9494 [3] protocolbuffers/protobuf#6711 [4] https://bugs.chromium.org/p/gerrit/issues/detail?id=11102

googlebot added the cla: yes label Sep 29, 2019

Bazel: Bump java source and target language level to java 8

75898c3

acozzette added release notes: yes java labels Sep 30, 2019

acozzette assigned rafi-kamal Sep 30, 2019

protobuf-kokoro removed the kokoro:run label Sep 30, 2019

rafi-kamal reviewed Oct 1, 2019

View reviewed changes

davido mentioned this pull request Oct 2, 2019

java_proto_library results in spurious warning message bazelbuild/bazel#8772

Closed

davido closed this Oct 3, 2019

davido mentioned this pull request Oct 3, 2019

Release 1.0 - September 2019 bazelbuild/bazel#8573

Closed

davido mentioned this pull request Oct 7, 2019

Protobuf: Remove java 7 compatibility mode bazelbuild/rules_closure#432

Merged

davido mentioned this pull request Mar 15, 2020

Restore protobuf patch to drop java 7 compatibility bazelbuild/rules_closure#478

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bazel: Bump java source and target language level to java 8 #6711

Bazel: Bump java source and target language level to java 8 #6711

davido commented Sep 29, 2019

rafi-kamal Oct 1, 2019

davido Oct 2, 2019

rafi-kamal Oct 2, 2019

davido Oct 2, 2019 •

edited

Loading

rafi-kamal Oct 2, 2019 •

edited

Loading

davido Oct 3, 2019

davido commented Oct 3, 2019

davido commented Mar 15, 2020 •

edited

Loading

Yannic commented Mar 15, 2020

davido commented Mar 15, 2020 •

edited

Loading

davido commented Mar 15, 2020

Yannic commented Mar 15, 2020

Bazel: Bump java source and target language level to java 8 #6711

Bazel: Bump java source and target language level to java 8 #6711

Conversation

davido commented Sep 29, 2019

rafi-kamal Oct 1, 2019

Choose a reason for hiding this comment

davido Oct 2, 2019

Choose a reason for hiding this comment

rafi-kamal Oct 2, 2019

Choose a reason for hiding this comment

davido Oct 2, 2019 • edited Loading

Choose a reason for hiding this comment

rafi-kamal Oct 2, 2019 • edited Loading

Choose a reason for hiding this comment

davido Oct 3, 2019

Choose a reason for hiding this comment

davido commented Oct 3, 2019

davido commented Mar 15, 2020 • edited Loading

Yannic commented Mar 15, 2020

davido commented Mar 15, 2020 • edited Loading

davido commented Mar 15, 2020

Yannic commented Mar 15, 2020

davido Oct 2, 2019 •

edited

Loading

rafi-kamal Oct 2, 2019 •

edited

Loading

davido commented Mar 15, 2020 •

edited

Loading

davido commented Mar 15, 2020 •

edited

Loading